Skip to content

feat: add X-Permit-Consistent-Update header on facts proxy requests#306

Open
omer9564 wants to merge 3 commits intomainfrom
omer/per-14392-consistent-updates-pdp-header
Open

feat: add X-Permit-Consistent-Update header on facts proxy requests#306
omer9564 wants to merge 3 commits intomainfrom
omer/per-14392-consistent-updates-pdp-header

Conversation

@omer9564
Copy link
Copy Markdown
Contributor

Summary

  • Adds the X-Permit-Consistent-Update: true header on facts requests that the PDP proxies through forward_request_then_wait_for_update (the wait-for-local-sync flow)
  • The backend's opal-interface uses this header to skip sending the control-plane delta update back to PDPs, since the PDP already propagates the update via OPAL Server pubsub after a successful write — removing a duplicate update path
  • The fallback proxy route (forward_remaining_requests) does NOT set the header, so generic passthrough requests continue to rely on the standard control-plane delta path

Details

The header is gated behind a new is_consistent_update: bool = False kwarg on FactsClient.build_forward_request and FactsClient.send_forward_request. Only forward_request_then_wait_for_update (called by the explicit consistent-update routes: users, tenants, role_assignments, resource_instances, relationship_tuples) passes True.

Paired with backend changes in permit-backend (branch: omer/per-14392-consistent-updates-duplicated-updates) which:

  1. Adds a FastAPI dependency that reads this header on all facts routes
  2. Injects is_consistent_update: True into the DB session's AMQP headers
  3. Makes opal-interface skip the delta publish when this flag is set

Test plan

  • New unit tests for FactsClient.build_forward_request:
    • Verifies header is set when is_consistent_update=True
    • Verifies header is NOT set by default (fallback proxy path)
  • All 47 horizon tests pass locally
  • Pre-commit clean (ruff, ruff-format)
  • Manual end-to-end: with backend change deployed, verify a proxied write results in a single update reaching the PDP (not two)

🤖 Generated with Claude Code

@linear
Copy link
Copy Markdown

linear Bot commented Apr 13, 2026

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 13, 2026

🔍 Vulnerabilities of permitio/pdp-v2:next

📦 Image Reference permitio/pdp-v2:next
digestsha256:6434e121c5fcf51c63f670e47555127f5871518f18a378ae720cbf9c4e08d84d
vulnerabilitiescritical: 0 high: 2 medium: 3 low: 0
platformlinux/amd64
size214 MB
packages252
📦 Base Image python:3.10-alpine3.22
also known as
  • 3.10.20-alpine3.22
digestsha256:c8f94b3bb77e6ea9015ccd091b7f8aec1b1fcbca95159675235d9a93788797cd
vulnerabilitiescritical: 0 high: 2 medium: 3 low: 1
critical: 0 high: 1 medium: 0 low: 0 sqlite 3.49.2-r1 (apk)

pkg:apk/alpine/sqlite@3.49.2-r1?os_name=alpine&os_version=3.22

high : CVE--2025--70873

Affected range<=3.49.2-r1
Fixed versionNot Fixed
EPSS Score0.037%
EPSS Percentile11th percentile
Description
critical: 0 high: 1 medium: 0 low: 0 go.opentelemetry.io/otel/sdk 1.42.0 (golang)

pkg:golang/go.opentelemetry.io/otel/sdk@1.42.0

high 7.3: CVE--2026--39883 Untrusted Search Path

Affected range>=1.15.0
<=1.42.0
Fixed version1.43.0
CVSS Score7.3
CVSS VectorCVSS:4.0/AV:L/AC:H/AT:N/PR:L/UI:N/VC:H/VI:H/VA:H/SC:N/SI:N/SA:N
EPSS Score0.006%
EPSS Percentile0th percentile
Description

Summary

The fix for GHSA-9h8m-3fm2-qjrq (CVE-2026-24051) changed the Darwin ioreg command to use an absolute path but left the BSD kenv command using a bare name, allowing the same PATH hijacking attack on BSD and Solaris platforms.

Root Cause

sdk/resource/host_id.go line 42:

if result, err := r.execCommand("kenv", "-q", "smbios.system.uuid"); err == nil {

Compare with the fixed Darwin path at line 58:

result, err := r.execCommand("/usr/sbin/ioreg", "-rd1", "-c", "IOPlatformExpertDevice")

The execCommand helper at sdk/resource/host_id_exec.go uses exec.Command(name, arg...) which searches $PATH when the command name contains no path separator.

Affected platforms (per build tag in host_id_bsd.go:4): DragonFly BSD, FreeBSD, NetBSD, OpenBSD, Solaris.

The kenv path is reached when /etc/hostid does not exist (line 38-40), which is common on FreeBSD systems.

Attack

  1. Attacker has local access to a system running a Go application that imports go.opentelemetry.io/otel/sdk
  2. Attacker places a malicious kenv binary earlier in $PATH
  3. Application initializes OpenTelemetry resource detection at startup
  4. hostIDReaderBSD.read() calls exec.Command("kenv", ...) which resolves to the malicious binary
  5. Arbitrary code executes in the context of the application

Same attack vector and impact as CVE-2026-24051.

Suggested Fix

Use the absolute path:

if result, err := r.execCommand("/bin/kenv", "-q", "smbios.system.uuid"); err == nil {

On FreeBSD, kenv is located at /bin/kenv.

critical: 0 high: 0 medium: 1 low: 0 sqlparse 0.5.0 (pypi)

pkg:pypi/sqlparse@0.5.0

medium 6.9: GHSA--27jp--wm6q--gp25 Allocation of Resources Without Limits or Throttling

Affected range<=0.5.3
Fixed version0.5.4
CVSS Score6.9
CVSS VectorCVSS:4.0/AV:N/AC:L/AT:N/PR:N/UI:N/VC:N/VI:L/VA:N/SC:N/SI:N/SA:N
Description

Summary

The below gist hangs while attempting to format a long list of tuples.

This was found while drafting a regression test for Dja
ngo 5.2's composite primary key feature
, which allows querying composite fields with tuples.

critical: 0 high: 0 medium: 1 low: 0 go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp 1.42.0 (golang)

pkg:golang/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp@1.42.0

medium 5.3: CVE--2026--39882 Memory Allocation with Excessive Size Value

Affected range<1.43.0
Fixed version1.43.0
CVSS Score5.3
CVSS VectorCVSS:3.1/AV:A/AC:H/PR:N/UI:N/S:U/C:N/I:N/A:H
EPSS Score0.020%
EPSS Percentile5th percentile
Description

overview:
this report shows that the otlp HTTP exporters (traces/metrics/logs) read the full HTTP response body into an in-memory bytes.Buffer without a size cap.

this is exploitable for memory exhaustion when the configured collector endpoint is attacker-controlled (or a network attacker can mitm the exporter connection).

severity

HIGH

not claiming: this is a remote dos against every default deployment.
claiming: if the exporter sends traces to an untrusted collector endpoint (or over a network segment where mitm is realistic), that endpoint can crash the process via a large response body.

callsite (pinned):

  • exporters/otlp/otlptrace/otlptracehttp/client.go:199
  • exporters/otlp/otlptrace/otlptracehttp/client.go:230
  • exporters/otlp/otlpmetric/otlpmetrichttp/client.go:170
  • exporters/otlp/otlpmetric/otlpmetrichttp/client.go:201
  • exporters/otlp/otlplog/otlploghttp/client.go:190
  • exporters/otlp/otlplog/otlploghttp/client.go:221

permalinks (pinned):

root cause:
each exporter client reads resp.Body using io.Copy(&respData, resp.Body) into a bytes.Buffer on both success and error paths, with no upper bound.

impact:
a malicious collector can force large transient heap allocations during export (peak memory scales with attacker-chosen response size) and can potentially crash the instrumented process (oom).

affected component:

  • go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp
  • go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp
  • go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp

repro (local-only):

unzip poc.zip -d poc
cd poc
make canonical resp_bytes=33554432 chunk_delay_ms=0

expected output contains:

[CALLSITE_HIT]: otlptracehttp.UploadTraces::io.Copy(resp.Body)
[PROOF_MARKER]: resp_bytes=33554432 peak_alloc_bytes=118050512

control (same env, patched target):

unzip poc.zip -d poc
cd poc
make control resp_bytes=33554432 chunk_delay_ms=0

expected control output contains:

[CALLSITE_HIT]: otlptracehttp.UploadTraces::io.Copy(resp.Body)
[NC_MARKER]: resp_bytes=33554432 peak_alloc_bytes=512232

attachments: poc.zip (attached)

PR_DESCRIPTION.md

attack_scenario.md

poc.zip

Fixed in: open-telemetry/opentelemetry-go#8108

critical: 0 high: 0 medium: 1 low: 0 busybox 1.37.0-r20 (apk)

pkg:apk/alpine/busybox@1.37.0-r20?os_name=alpine&os_version=3.22

medium : CVE--2025--60876

Affected range<=1.37.0-r20
Fixed versionNot Fixed
EPSS Score0.051%
EPSS Percentile16th percentile
Description

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 13, 2026

🔍 Vulnerabilities of permitio/pdp-v2:next

📦 Image Reference permitio/pdp-v2:next
digestsha256:6434e121c5fcf51c63f670e47555127f5871518f18a378ae720cbf9c4e08d84d
vulnerabilitiescritical: 0 high: 2 medium: 0 low: 0
platformlinux/amd64
size214 MB
packages252
📦 Base Image python:3.10-alpine3.22
also known as
  • 3.10.20-alpine3.22
digestsha256:c8f94b3bb77e6ea9015ccd091b7f8aec1b1fcbca95159675235d9a93788797cd
vulnerabilitiescritical: 0 high: 2 medium: 3 low: 1
critical: 0 high: 1 medium: 0 low: 0 sqlite 3.49.2-r1 (apk)

pkg:apk/alpine/sqlite@3.49.2-r1?os_name=alpine&os_version=3.22

high : CVE--2025--70873

Affected range<=3.49.2-r1
Fixed versionNot Fixed
EPSS Score0.037%
EPSS Percentile11th percentile
Description
critical: 0 high: 1 medium: 0 low: 0 go.opentelemetry.io/otel/sdk 1.42.0 (golang)

pkg:golang/go.opentelemetry.io/otel/sdk@1.42.0

high 7.3: CVE--2026--39883 Untrusted Search Path

Affected range>=1.15.0
<=1.42.0
Fixed version1.43.0
CVSS Score7.3
CVSS VectorCVSS:4.0/AV:L/AC:H/AT:N/PR:L/UI:N/VC:H/VI:H/VA:H/SC:N/SI:N/SA:N
EPSS Score0.006%
EPSS Percentile0th percentile
Description

Summary

The fix for GHSA-9h8m-3fm2-qjrq (CVE-2026-24051) changed the Darwin ioreg command to use an absolute path but left the BSD kenv command using a bare name, allowing the same PATH hijacking attack on BSD and Solaris platforms.

Root Cause

sdk/resource/host_id.go line 42:

if result, err := r.execCommand("kenv", "-q", "smbios.system.uuid"); err == nil {

Compare with the fixed Darwin path at line 58:

result, err := r.execCommand("/usr/sbin/ioreg", "-rd1", "-c", "IOPlatformExpertDevice")

The execCommand helper at sdk/resource/host_id_exec.go uses exec.Command(name, arg...) which searches $PATH when the command name contains no path separator.

Affected platforms (per build tag in host_id_bsd.go:4): DragonFly BSD, FreeBSD, NetBSD, OpenBSD, Solaris.

The kenv path is reached when /etc/hostid does not exist (line 38-40), which is common on FreeBSD systems.

Attack

  1. Attacker has local access to a system running a Go application that imports go.opentelemetry.io/otel/sdk
  2. Attacker places a malicious kenv binary earlier in $PATH
  3. Application initializes OpenTelemetry resource detection at startup
  4. hostIDReaderBSD.read() calls exec.Command("kenv", ...) which resolves to the malicious binary
  5. Arbitrary code executes in the context of the application

Same attack vector and impact as CVE-2026-24051.

Suggested Fix

Use the absolute path:

if result, err := r.execCommand("/bin/kenv", "-q", "smbios.system.uuid"); err == nil {

On FreeBSD, kenv is located at /bin/kenv.

@omer9564 omer9564 requested review from Zivxx, Copilot and zeevmoney April 14, 2026 08:41
omer9564 and others added 2 commits April 14, 2026 11:41
The backend opal-interface uses this header to skip sending the
control-plane delta update back to PDPs, since the PDP already
propagates the update via OPAL Server pubsub after a successful
facts proxy write.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move the X-Permit-Consistent-Update header injection behind an
explicit is_consistent_update kwarg so the fallback proxy route
(forward_remaining_requests) does not falsely mark generic
passthrough requests as consistent updates.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@omer9564 omer9564 force-pushed the omer/per-14392-consistent-updates-pdp-header branch from af90b4f to 74bf9ed Compare April 14, 2026 08:41
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an opt-in request header to facts-service proxy calls made via the “wait-for-local-sync” path, allowing the backend to skip emitting a redundant control-plane delta update when the PDP is already publishing an OPAL pubsub update.

Changes:

  • Introduce CONSISTENT_UPDATE_HEADER and an is_consistent_update kwarg on FactsClient.build_forward_request() / send_forward_request(), adding X-Permit-Consistent-Update: true when enabled.
  • Set is_consistent_update=True for the consistent-update proxy flow (forward_request_then_wait_for_update).
  • Add unit tests asserting the header is present when requested and absent by default.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
horizon/facts/client.py Adds the consistent-update header constant and gating logic in forwarded request construction/sending.
horizon/facts/router.py Enables the consistent-update header for the wait-for-local-sync proxy flow.
horizon/tests/test_facts_client.py Adds tests covering header inclusion/exclusion behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread horizon/facts/router.py
Copy link
Copy Markdown
Contributor

@zeevmoney zeevmoney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, small issues.

Comment thread horizon/tests/test_facts_client.py Outdated
Comment thread horizon/facts/router.py
Comment thread horizon/facts/client.py
Comment thread horizon/facts/client.py Outdated
Address review feedback on PR #306:
- Extract CONSISTENT_UPDATE_HEADER_VALUE constant for the "true" literal
- Replace tautological constant test with literal-header-spelling assertion
- Add send_forward_request kwarg passthrough test
- Add router-level tests pinning is_consistent_update on wait-for-update
  path and asserting the fallback proxy does not set the header

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants